chore: internal audit remediation (25 tasks + security fix + ruff sweep) - #327

Tasks 1-4, 20: Add governance files, configuration templates, and packaging improvements.

Task 1: Update icon and logo set (14 assets: multiple resolutions and formats).
Task 2: Add CODE_OF_CONDUCT.md (Contributor Covenant v2.1).
Task 3: Add .editorconfig for editor/IDE settings alignment.
Task 4: Add py.typed marker (PEP 561), fix stale license-files entry in pyproject.toml,
        pin hatchling>=1.26 build-system floor, add packaging tests.
Task 20: Add .env.example documenting every MCPG_* configuration variable.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…2SQL_API_KEY

- Replace CODE_OF_CONDUCT.md body with verbatim Contributor Covenant v2.1 from authoritative source
  (was paraphrased before). Fixes: missing 'caste, color' in protected characteristics,
  'We pledge that we will' → 'We pledge to' per spec, double space in line 109.
  Enforcement contact remains devopam@gmail.com (matches SECURITY.md).
- Add MCPG_NL2SQL_API_KEY to .env.example NL→SQL section (was omitted despite being
  read via secrets.get() in config.py:870 and documented in docs/user-guide.md:520).
- Verified completeness: all 93 MCPG_* variables from config.py now in .env.example.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…or-logging call sites

Task 6: adds a logger.debug(..., exc_info=True) call to 7 previously-silent
except Exception: pass sites (advisors.py, audit.py, audit_trail.py x3,
listen.py, migrations.py). Zero control-flow change - migrations.py keeps
its trailing raise unchanged.

Task 14: audited every logger.error/warning call inside an except block
across src/mcpg (33 candidate sites found via fresh grep, not the earlier
non-exhaustive 1-2-hit sample). 23 sites needed exc_info=True added
(audit_nl2sql.py, cache.py, cursors.py, database.py, graph_diagram.py,
http_runtime.py, nl2sql.py, otel_tracing.py, replicas.py, tenancy.py).
1 site already had exc_info=True. 9 sites left unchanged - either not
inside an except block, or logging about something other than the
caught exception.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

… 7 traceback tests

Review found two Important issues in the prior commit (fe95e70):

1. exc_info=True attaches a formatted traceback whose last line is
   "ExceptionType: str(exception)", bypassing obfuscate_password()
   applied to the message argument. Reverted exc_info=True at the 5
   sites where the caught exception can carry a DSN/connection string
   and the message was already redacted for exactly that reason:
   database.py:117, replicas.py:139, replicas.py:353, nl2sql.py:1077,
   nl2sql.py:1081. All 5 are now byte-identical to their pre-batch
   state. The other 18 Task-14 fixes and all 7 Task-6 fixes are
   untouched.

2. None of the 7 Task-6 caplog tests verified that exc_info was
   actually attached - only message text + level, so dropping
   exc_info=True while keeping the message would have stayed green.
   Strengthened all 7 to also assert record.exc_info is not None.
   Verified the new assertion actually catches the regression by
   temporarily removing exc_info=True from advisors.py and confirming
   the test failed, then restoring it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

Distinct from /healthz (liveness): /readyz reports 503 when the DB
pool hasn't produced a live connection (Database.is_connected), so an
orchestrator can pull a degraded instance out of rotation without
restarting it. Already reserved in _AUTH_EXEMPT_PATHS but never
mounted.

_health_response_factory has no pool-access pattern to reuse - the DB
pool lives in AppContext, delivered by the MCP SDK's lifespan only
during tool-call handling, not exposed on app.state or to a bare
Starlette Route. create_server now stashes the primary Database on
the server object (server.mcpg_database) alongside the existing
mcpg_settings/otel_tracer/rate_limiter, so build_http_app can read it
via getattr and pass it into the new _readiness_response_factory.

OIDC/JWKS readiness gating was considered and deliberately omitted:
/readyz must stay auth-exempt, so gating it on "JWKS fetched via a
verified request" would deadlock a fresh OIDC instance out of
rotation before it ever receives the traffic needed to populate the
cache.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

run_select / run_select_tuned fully materialized a query's entire result
set into Python dict/RowResult objects before truncating to max_rows.
SqlDriver.execute_query / SafeSqlDriver.execute_query (and every
SqlDriver subclass override in multidb.py/replicas.py/tenancy.py) now
take an optional row_limit parameter and use cursor.fetchmany(row_limit)
instead of cursor.fetchall() when set; both call sites in query.py pass
max_rows + 1.

Note on scope: psycopg's client-side cursor already pulls the whole
result set into libpq's buffer during cursor.execute(), so this bounds
Python-side object materialization, not the server-side network
transfer. A true wire-level bound would need a named server-side cursor
or an injected LIMIT, which can't cleanly support run_select_tuned's
SET LOCAL ...; SELECT ... pattern or the SHOW/EXPLAIN/VACUUM paths
SafeSqlDriver also allows -- flagged as a possible follow-up, not
built here.

_validate/_validate_node and allowlist.py are untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

BREAKING CHANGE: the HTTP transport now raises ConfigError at startup instead of starting
unauthenticated-with-a-warning when neither MCPG_HTTP_AUTH_TOKEN nor MCPG_AUTH_MODE=oidc is
set. Set MCPG_HTTP_ALLOW_UNAUTHENTICATED=true to explicitly opt out.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

BREAKING CHANGE: MCPG_RATE_LIMIT_ENABLED now defaults to true. Set it to false explicitly to
restore the previous unlimited default.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…eware

- HSTS max-age default bumped 31536000 (1yr) -> 63072000 (2yr, OWASP's
  current recommendation) in both the Settings dataclass default and the
  load_settings parse default (and the _SecurityHeadersMiddleware
  constructor default for consistency).
- New Settings.http_trusted_hosts: tuple[str, ...] = () field, following
  the exact http_allowed_origins comma-split-env-var / empty-tuple-means-off
  pattern. MCPG_HTTP_TRUSTED_HOSTS wires Starlette's TrustedHostMiddleware
  conditionally in build_http_app, mirroring the existing CORS conditional.
- Updated README.md / .env.example to reflect the new HSTS default and
  document MCPG_HTTP_TRUSTED_HOSTS.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…rifier

NL2SQL providers (AnthropicProvider/OpenAIProvider/GeminiProvider) each
opened `async with httpx.AsyncClient(...)` fresh on every `complete()`
call, paying a full TCP/TLS handshake per translate_nl_to_sql
invocation. build_provider() is called fresh on every tool call
(provider selection varies per-request via `provider=`), so holding a
client on the provider *instance* wouldn't help -- a new provider
still means a new client. Instead, nl2sql.py now holds one
lazily-constructed httpx.AsyncClient at module scope, shared by all
three provider classes (httpx already pools connections per-host
internally, so sharing one client across vendors is correct usage).
Closed via aclose_shared_client(), wired into make_lifespan's existing
shutdown path in server.py.

OIDCVerifier's discovery-document fetch had the same per-call client
problem. OIDCVerifier IS constructed once per server lifetime (in
http_runtime.build_http_app), so it now holds one httpx.AsyncClient in
__init__ and a new aclose(). Since the verifier is built after
make_lifespan's closure already exists (build_http_app runs later,
inside run_http), that closure can't reach it -- build_http_app
instead wraps the Starlette app's own ASGI lifespan_context so the
verifier's client still gets closed on shutdown, for both the
streamable-http and sse transports.

Regression tests exercise the real per-call construction pattern (two
separate build_provider() calls / two full ASGI lifespan cycles)
rather than two .complete() calls on one hand-held provider instance,
which would pass even against the unfixed per-call-client code.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…calls

Wraps each NL2SQL provider's complete() (AnthropicProvider, OpenAIProvider,
GeminiProvider) and OIDCVerifier._resolve_jwks_url (the discovery-document
fetch) with circuitbreaker.circuit — 5 consecutive failures opens the
breaker for 30s, so a degraded vendor/IdP fails fast instead of every
request separately paying the full timeout.

circuitbreaker 2.1.3 supports async natively (iscoroutinefunction check
dispatches to an async-aware wrapper) — confirmed by reading the installed
package's source, not assumed from the audit report's secondary
characterization. Parameter names failure_threshold/recovery_timeout/
expected_exception match the brief.

@circuit decorates the plain function at class-definition time, so its
failure count is one object shared by every instance of that class for the
process's lifetime, not per-instance state. This is intentional for the
NL2SQL providers: build_provider() constructs a fresh provider on every
translate_nl_to_sql call, so per-instance state would never accumulate a
failure across calls.

A tripped breaker raises circuitbreaker.CircuitBreakerError, not the
module's own error type — both call sites (translate_nl_to_sql's except
around provider.complete(), OIDCVerifier._ensure_jwks_client's call to
_resolve_jwks_url) now also catch CircuitBreakerError and re-raise it as
NL2SQLError / OIDCError respectively, so existing callers (including
http_runtime.py's `except OIDCError` around verifier.verify) keep working
unchanged whether the breaker is open or not.

mypy --strict's disallow_untyped_decorators fires on @circuit independently
of import resolution (circuitbreaker ships no py.typed marker); each
decorated method carries a `# type: ignore[untyped-decorator]` alongside a
new ignore_missing_imports override for the module.

Tests reset every registered circuit breaker via an autouse fixture in both
test_nl2sql.py and test_oidc.py — the breaker's shared-at-class-level state
would otherwise leak across unrelated tests in the same session.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…KS calls

Layers tenacity's @Retry INSIDE (not outside) the @circuit breaker added in
the prior commit, on the same four call sites (AnthropicProvider,
OpenAIProvider, GeminiProvider .complete(); OIDCVerifier._resolve_jwks_url):

    @circuit(...)   # outer
    @Retry(...)     # inner
    async def complete(self, ...): ...

This ordering is verified against circuitbreaker's actual failure-counting
semantics (read from the installed package's source in the prior commit):
`call_async` does `with self: return await func(...)` and counts exactly
one failure per invocation of whatever it wraps. With retry innermost, all
of a logical call's retry attempts happen inside that single `with self:`
block, so an exhausted retry cycle counts as ONE breaker failure — not one
per retry. Stacked the other way (retry outer, circuit inner — the shape a
stale draft of this task's brief showed), each retry attempt would
separately enter/exit the breaker's `with self:`, letting 3 quick retries
alone burn 3 of the breaker's failure_threshold slots for a single logical
call. The task's own top-level framing used the word "outside" for this
same intended behavior ("only the whole retried attempt counts as one
failure toward the breaker's threshold") while showing retry-outer code —
those two are inconsistent for this library; the code matches the stated
behavior, not the word.

`reraise=True` is load-bearing, not cosmetic: without it, an exhausted
retry cycle raises tenacity.RetryError, which wouldn't match @circuit's
expected_exception (httpx.HTTPError / OIDCError) — the breaker would
silently never count a single failure. `retry_if_exception_type` scopes
retries to the same exception type each breaker already keys on, so a
non-network bug can't accidentally get retried either.

Backoff is small and jittered (wait_exponential_jitter, initial=0.1s,
max=2s, jitter=0.1s, 3 attempts) — the breaker, not the backoff, is the
mechanism that handles a sustained outage, so there's no reason to make a
doomed request wait long between retries.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…path

The `except CircuitBreakerError` branch added to `translate_nl_to_sql` in
the circuit-breaker commit (e916ff4) was never exercised by any test — a
coverage run confirmed nl2sql.py:1360-1365 as "Missing." The existing
circuit-breaker tests call `AnthropicProvider(...).complete(...)` directly,
bypassing `translate_nl_to_sql` entirely, so the translation this batch's
own CHANGELOG entry advertises ("a tripped breaker still surfaces as the
module's existing error type… never a bare CircuitBreakerError") was
asserted on the OIDC side only, not on NL2SQL's.

Adds a test that trips the breaker via translate_nl_to_sql itself (not
provider.complete() directly) and asserts the next call raises
NL2SQLError, not a bare CircuitBreakerError. No production code changed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

…on test

Address task-13 review findings: add a test that logs through the real
setup_logging() -> handler -> JSONFormatter pipeline (not just
RedactionFilter/JSONFormatter in isolation), and narrow the docstring and
CHANGELOG wording to accurately state that record.exc_info (exception
tracebacks) is not covered by this filter.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…rge check

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

The comment landed inside the YAML block scalar (if: |), which has no
comment syntax -- every line at or above its indentation becomes literal
content of the string GitHub Actions evaluates as the job condition.
Moved the explanatory comment above the if: key as ordinary YAML
comments, outside the block scalar. Verified the parsed if string is
byte-identical to commit 05f09f1's value.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…tion

describe_graph (graph.py) and generate_graph_diagram (graph_diagram.py)
read Apache AGE vertex/edge label names back from ag_catalog.ag_label
and interpolated them directly into f-string SQL (`FROM "{graph}"."{label}"`)
with no identifier validation. Postgres/AGE quoted identifiers can contain
arbitrary characters, including embedded double quotes, so a label name
created via a prior run_cypher write could carry attacker-controlled SQL
into that later re-interpolation.

Fix:
- Every catalog-derived label name is identifier-validated
  ([A-Za-z_][A-Za-z0-9_]*) before it reaches a generated SQL string or the
  diagram's Mermaid output (new mcpg.graph._check_label_identifier,
  mirroring graph_projection._check_identifier). An invalid name aborts
  the call with GraphError rather than being silently used or skipped,
  matching graph_projection's existing precedent for catalog-derived
  identifiers.
- Both tools' per-label count/fetch queries now run under
  force_readonly=True.
- describe_graph gained the same Capability.READ access-mode gate its
  sibling read tools (run_cypher's read path, generate_graph_diagram)
  already had, for consistency — Capability.READ is currently permitted
  in every access mode, so this is defense-in-depth/consistency, not a
  fix for a live privilege bypass (generate_graph_diagram already had
  this gate; the original finding's "access-mode bypass" framing for it
  did not hold up against the current code).

Tests (RED/GREEN): new tests in test_graph.py and test_graph_diagram.py
construct a label name containing an embedded double-quote/`;` and
confirm the pre-fix code either builds the unsafe SQL text or silently
degrades, while post-fix it raises GraphError before any such query
reaches the driver; a monkeypatch spy confirms check_permission is
invoked. Verified against the pre-fix code via git stash before applying
the fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

…lues

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…d values

Replaced cosmetic rewrites with genuine strengthening:
1. test_demo.py: assert product name or features are interpolated in review text
2-4. test_http_runtime.py: assert middleware passes scope/receive/send unmodified
5. test_warehousepg_reads.py: assert exactly 1 query, not 2+

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…chine, pytest-rerunfailures

Full unit suite (2950 tests) run twice under pytest-randomly's order
randomization with different seeds -- both runs passed with no
order-dependence failures. pytest-socket is added but not globally
enabled (no --disable-socket, no conftest.py changes).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Rescoped ruff sweep, Task 23 Steps 1-3 + the PYI sub-step: fix all
pre-existing violations in four small, safe, high-value opt-in Ruff
categories, run via explicit --select flags (pyproject.toml's
[tool.ruff.lint] select list is untouched -- that lands in a later,
final commit once C90/ASYNC/FBT are also fixed).

- PTH (flake8-use-pathlib), 9 violations: os.path.* -> pathlib.Path,
  open() -> Path.open(). Every hit hand-reviewed for symlink/relative-
  path behavior preservation, including the subprocess bin-allowlist
  symlink check in shell.py (only os.path.dirname() -> Path(...).parent
  changed; the os.path.realpath() symlink-resolution call is untouched).
- C4 (flake8-comprehensions), 1 violation: unnecessary list-in-tuple().
- SIM (flake8-simplify), 48 violations: 20 autofixed (yoda-conditions,
  if-with-same-arms), 28 manual (contextlib.suppress for try/except/
  pass, if/else-to-ternary, nested-if collapsing, with-statement
  merging). Includes two fixes in the security-critical SQL-safety AST
  walker (sql/safety.py) -- confirmed behavior-preserving against the
  full adversarial + fuzz suite. One SIM103 hit (pgq.py) was NOT applied
  as ruff's literal suggestion because it would have widened the
  function's declared bool return type; wrapped in bool(...) instead to
  preserve both behavior and the mypy --strict contract.
- PYI (flake8-pyi), 16 violations: 2 autofixed (redundant float|int
  union), 14 manual (__aenter__/__enter__ return type -> Self).

Full verification: ruff check . -> 0; ruff format --check . -> clean;
mypy src/mcpg --strict -> 0 issues; tests/unit (what the pre-commit
hook gates) -> 2950 passed, 3 skipped, 0 failed. One pre-existing,
unrelated contract-test failure in tests/contract/ (a
TranslationResult.schema_context snapshot drift from an earlier,
already-merged commit) is out of this batch's scope and does not touch
tests/unit -- flagged for a follow-up snapshot regeneration.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

…ext field

The schema_context field added to TranslationResult (commit 6d55775) was a
legitimate, intentional dataclass change but the tool-return-shapes contract
snapshot was never regenerated for it, leaving the contract test red outside
the pre-commit hook's tests/unit-only scope. Regenerated per the project's own
documented process (MCPG_REGENERATE_TOOL_RETURN_SHAPES=1).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Reviews the remaining two opt-in ruff categories from the audit-remediation
sweep -- ASYNC (flake8-async, 18 violations) and C901 (mccabe complexity,
66) -- individually rather than blanket-fixing or blanket-suppressing.
pyproject.toml's [tool.ruff.lint] select list is still untouched; only a
new `external = ["ASYNC", "C90"]` entry was added so this task's mandated
per-violation `# noqa` justifications don't trip RUF100 (unused-noqa)
under a bare `ruff check .` before a later step formally enables the
categories.

ASYNC (18/18 justified-suppressed, 0 code changes needed):
- All 14 ASYNC109 ("timeout" parameter) hits are pass-throughs to a lower
  layer that already owns real timeout enforcement -- httpx's own
  per-request timeout (nl2sql.py's LLMProvider implementations,
  observe_llm_behaviour.py) or SafeSqlDriver's existing
  asyncio.timeout() (query.py's run_select/run_select_tuned/
  explain_query/analyze_query_plan/run_select_parallel, confirmed by
  reading sql/safety.py:181).
- The 3 ASYNC240 + 1 ASYNC221 blocking-call hits are all test-only or
  one-off-script call sites (test_shell.py assertions after the call
  under test returned, a one-time pg_dump --version probe, a benchmark
  script's teardown unlink), not hot paths.

C901 (7 refactored, 59 justified-suppressed):
- Refactored (cheap, safe, test-verified): diesel.generate_diesel_schema,
  sqlc.generate_sqlc_schema (extract-a-section-into-a-helper),
  test_row_factory._synth_by_name / _synth_by_type (if-chain to
  predicate/generator dispatch tables), test_data.generate_test_data
  (extract per-column value formatting), audit.audit_database (collapse
  a 6-branch "if not None: append" chain into one filtered extend),
  diagrams.generate_schema_diagram (extract entity-block /
  relationship-line rendering).
- Suppressed with an individual one-line reason each: the security-
  critical SQL-safety kernel (sql/safety.py's _validate_node, per this
  task's own mandate -- citing the module's existing fuzz-tested /
  adversarially-pinned rationale, not refactored; sql/driver.py's
  _execute_with_connection; tenancy.py's _execute_with_role), other
  identifier-validation / credential-handling / subprocess-sandboxing /
  tamper-evidence paths, the tools.py MCP-tool-registration functions
  gated by the frozen 254-tool contract snapshot, numerical algorithms
  (k-means, MMR re-ranking, ANN recall sweeps) where restructuring risks
  changing the computed result, config.load_settings (complexity 178,
  the single process-wide env-var reader), and several schema-generator
  functions in the same family as the refactored diesel.py/sqlc.py but
  with cross-cutting state that resists the same cheap extraction.

Full per-function detail:
.superpowers/sdd/2026-08-25-audit-remediation/batch-ruffB-report.md

Verification: `ruff check --select ASYNC,C901 .` -> 0. Full sweep --
`ruff check . && ruff format --check . && mypy src/mcpg && pytest -q`
(tests/unit + tests/contract + tests/integration in one run) -- clean,
3001 passed / 115 skipped, no failures, no tool-surface or
tool-return-shape contract drift.

Advances roadmap row: N/A (lint remediation, not a roadmap feature)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

…external's forward side-effect

Two review findings on the prior ASYNC/C901 assessment commit (2b3de26),
both documentation-of-a-correct-decision issues, no logic changes:

1. query.py:187's noqa comment claimed `run_select_tuned`'s timeout is
   enforced by SafeSqlDriver's asyncio.timeout() -- copy-pasted from the
   adjacent run_select/explain_query wording without checking that
   run_select_tuned only calls safe_driver._validate() (a pure pglast
   parse), never safe_driver.execute_query(). The SafeSqlDriver
   constructor's timeout= is dead on this path; real enforcement is the
   explicit asyncio.wait_for(..., timeout=timeout) around the actual
   execute_query call further down the same function (which already had
   a correct inline comment). Fixed the noqa comment, this task's batch
   report, and the matching CHANGELOG.md wording to name the real
   mechanism.

2. pyproject.toml's `external = ["ASYNC", "C90"]` (added so RUF100 doesn't
   flag this task's mandated per-violation noqa comments as unused before
   a later step enables these categories in `select`) prefix-matches
   unconditionally -- it will keep blinding RUF100 to stale ASYNC/C901
   noqa comments even after `select` is updated. Added an explicit
   removal reminder in the pyproject.toml comment itself, next to the
   entry, for whoever does that later select-list update.

Verification: `ruff check . && ruff format --check . && mypy src/mcpg`
all clean (comment-only fix; full suite not re-run per reviewer guidance).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

…s.py subset)

Fixes all 103 FBT (flake8-boolean-trap) violations in src/mcpg/tools.py —
the tools.py half of the 196-violation FBT sweep (Task 23, Step 9).

The 103 diagnostics (53 FBT001 + 50 FBT002; no FBT003) resolve to 53
boolean parameters across 43 functions. AST decorator inspection confirms
all 43 are registered @server.tool MCP tools — there were no internal
helper hits in this file, so no helper call sites needed updating.

A `*` marker is inserted before each signature's first boolean parameter
rather than reordering parameters, preserving declaration order and hence
JSON-Schema `required` order.

The exposed MCP wire contract is UNCHANGED. The mcp SDK derives each
tool's inputSchema via pydantic, which treats keyword-only and
positional-or-keyword parameters identically. Verified by asserting
before regenerating: tests/contract/ passes with no regeneration env var
set (51 passed), and re-running both MCPG_REGENERATE_TOOL_SNAPSHOT=1 and
MCPG_REGENERATE_TOOL_RETURN_SHAPES=1 reproduces both snapshots
byte-for-byte (empty git diff). No snapshot lines are committed — that is
the result, not an omission. MCP clients always dispatch tool arguments
by name and are unaffected.

Also passes per_record= by keyword into walinspect.read_pg_wal_stats.
Ruff's FBT003 only fires on boolean *literals*, not forwarded boolean
*variables*, so the pending ~93-hit batch outside tools.py could have
made that helper's parameter keyword-only and broken this caller with no
lint signal.

Verified: ruff check . clean; ruff format --check . clean;
mypy src/mcpg clean (109 files); pytest -q → 3001 passed, 115 skipped.

Advances roadmap row: n/a (lint remediation)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Fixes the remaining 30 FBT violations outside tests/ -- 26 in src/
(excluding tools.py, done in eab2ef0) and 4 in tools/ dev scripts.
tests/ is deliberately out of scope for this batch.

The substantive change is in the SQL-safety kernel: force_readonly is
now keyword-only across the whole execute_query / _execute_with_connection
override family (sql/driver.py, sql/safety.py, multidb.py, replicas.py,
tenancy.py), so a security-relevant read-only flag can no longer be
passed in the wrong positional slot. No allowlist, policy, or control
flow was touched -- only parameter kind and call-site syntax.

No public MCP tool signature and no snapshot changed; these are all
internal helpers and driver methods. tests/contract/ passes unmodified
(51 passed), and the full suite is 3001 passed / 115 skipped, matching
the pre-change baseline exactly.

Also fixes 10 positional boolean-forwarding hazards that ruff cannot
see -- FBT003 fires only on boolean literals, never on a boolean
variable forwarded positionally. Found by an AST arity sweep over
src/ + tools/ + tests/ (count cross-checked against ruff's own FBT001).
One of them, replicas.py's TenantTimeoutSqlDriver, would otherwise have
broken at runtime: its super() call was already multi-line, so a
single-line edit pass missed it and neither ruff, mypy (the override is
untyped), nor the test suite would have caught it.

Zero noqa added. pyproject.toml's [tool.ruff.lint] select list untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

The entry said "10 positional boolean-forwarding hazards"; the precise
figure is 28 call sites converted, of which ruff flagged only 3 (the
FBT003 literals in pitr.py). The other 25 are boolean variables
forwarded positionally, which FBT003 structurally cannot see.

Breakdown: force_readonly family 9, indexing 3, _validate_bool 8,
tools/ 2, PitrGate 6. Verified by AST arity sweep, not by the linter.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

Review (batch-ruffD-review.md) flagged as an actionable Minor: the
CHANGELOG entry never stated the real signature count (20 — 18
ruff-flagged plus 4 untyped subclass overrides converted for family
consistency), leaving the immutable commit subject's '18' uncorrected.
The report itself already reconciled 18 vs 20; this surfaces that
reconciliation in the reader-facing CHANGELOG too.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

…T, Part E)

Completes the FBT (flake8-boolean-trap) sweep across Parts C+D+E:
103 (tools.py) + 30 (src/ + tools/) + 126 (tests/) = 259 total violations
fixed, FBT is now clean repo-wide.

- 45 test-helper/fixture/fake definitions made keyword-only, every call
  site fixed to match.
- Completed the Part D handoff exactly as specified: tests/unit/_fakes.py's
  three standalone fakes (FakeDriver, FakeRoutingDriver,
  FakeParamRoutingDriver) had force_readonly made keyword-only together
  with all 5 positional call sites into them, in this same commit, verified
  by grep before and after.
- FBT003 call-site literals converted to keyword form, including several
  dataclass constructions (DomainInfo/PublicationInfo/SubscriptionInfo/
  ColumnInfo/CronJob) verified against their actual field order in src/.
  One *key_args cache-key call (test_multidb_cache.py) was rewritten to
  pass a named variable instead of a literal, clearing FBT003 without a
  noqa.
- Zero noqa added. One definition (test_database.py's _FakeConnection,
  mirroring psycopg's set_autocommit) had its bool annotation dropped
  rather than made keyword-only, since keyword-only would have required
  also changing its production caller (src/mcpg/database.py) -- out of
  this batch's scope. src/ is untouched by this commit.
- pyproject.toml's [tool.ruff.lint] select list intentionally not
  touched -- that is a separate later step (adding FBT/C90/ASYNC/C4/SIM/
  PTH/PYI) once all of Parts C/D/E are done.

Verification: uv run ruff check . clean; uv run mypy src/mcpg clean;
full suite 3001 passed/115 skipped; tests/contract/ 51 passed standalone
-- both identical to Part D's baseline (no regression).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

…(Task 23, Step 11)

Completes the rescoped ruff sweep (Task 23): all 7 categories fully
assessed/fixed to zero across Parts A-E are now enforced by a bare
ruff check ., not just spot-checked via --select. Removed the
external = ["ASYNC", "C90"] entry added during Part B (its own comment
mandated removal at this exact point) -- RUF100 now polices the ~113
ASYNC/C901 noqa comments normally; none were stale.

No [tool.ruff.lint.mccabe] section added: Part B's 66 C901 findings were
measured against ruff's unconfigured default max-complexity of 10 (several
findings at exactly complexity 11 corroborate this), so leaving it unset
preserves the exact bar already assessed.

One new FBT001 hit surfaced once FBT was actually selected, in
benchmarks/tokens/tier_b/experiments/real_harness_comparison.py's
_result_to_trial helper -- a scope gap (benchmarks/ was never covered by
Parts C/D/E's globs: tools.py, src/+tools/, tests/), not interim drift.
Fixed directly as a trivial single-call-site change (passed: bool -> *,
passed: bool) since it is not a registered MCP tool.

Combined total across Parts A-E: 417 violations addressed, verified
against .superpowers/sdd/2026-08-25-audit-remediation/progress.md.

Verification: ruff check . clean, ruff format --check . clean (338 files),
mypy src/mcpg clean (109 files), pytest -q 3001 passed/115 skipped,
pytest -q tests/contract/ 51 passed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj

Fix wave for the 4 Minor findings from the whole-branch final review
(.superpowers/sdd/2026-08-25-audit-remediation/final-review-report.md).
Doc/config-only, no behavior change.

- M-1: collapse the CHANGELOG's 6 overlapping ruff-sweep entries
  (Step 11, Part D, Part E, Part C, Part A, Part B) into one coherent
  Changed entry covering the categories added, the final totals (418
  violations: 74 + 84 + 260), the force_readonly keyword-only
  conversion, the two near-miss bugs the AST arity sweep caught, and
  the mccabe-default-10 justification. Granular per-part narrative
  moved out to batch-ruff*.md (already the source of truth there).

- M-2: re-measured the historical FBT remainder breakdown against the
  batch reports (not trusted from the old text): 26 in src/, 126 in
  tests/, 4 in tools/ (not 5), 1 in benchmarks/ (previously
  uncounted) = 157. Verified against batch-ruffD/E-report.md and
  batch-ruff-step11-report.md; current ruff check --select FBT... is
  0 across src/, tests/, tools/, benchmarks/.

- M-3: disclose in the Task 21 dependency entry that only
  pytest-randomly is active; pytest-mock, pytest-socket,
  time-machine, and pytest-rerunfailures are staged for future
  test-hygiene work and unused today (verified by grep). No
  dependency removed from pyproject.toml.

- M-4: docs/security-hardening.md's HSTS default was stale
  (31536000); updated to the current 63072000 default, matching
  README.md. Grepped docs/ for other stale 31536000 references;
  found none needing a fix (the plan doc's mention is a correct
  historical before-value).

Verification: ruff check . clean; full [Unreleased] section read for
coherence; pytest -q full suite 3001 passed / 115 skipped (matches
the review's baseline exactly).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013bghcqrffRB2LTwv2q2bTj